Skip to content

Use pull_request_target for e2e tests to support fork PRs#796

Open
kelos-bot[bot] wants to merge 2 commits intomainfrom
kelos-task-793
Open

Use pull_request_target for e2e tests to support fork PRs#796
kelos-bot[bot] wants to merge 2 commits intomainfrom
kelos-task-793

Conversation

@kelos-bot
Copy link

@kelos-bot kelos-bot bot commented Mar 26, 2026

/kind bug

What type of PR is this?

Bug fix — e2e tests cannot access secrets for PRs from forked repositories.

What this PR does / why we need it:

GitHub Actions does not expose repository secrets to pull_request events from forks. This PR switches the e2e test job to use pull_request_target instead, which runs in the base repository's context and has access to secrets.

Changes:

  • Add pull_request_target trigger to the CI workflow (restricted to labeled type only, so maintainers must re-apply ok-to-test after each new fork commit)
  • Include event_name in the concurrency group key to prevent pull_request and pull_request_target runs from cancelling each other
  • Non-e2e jobs (build, verify, test, test-integration) skip pull_request_target events to avoid double runs
  • E2e job now triggers on pull_request_target (with ok-to-test label gate) instead of pull_request
  • Checkout step in e2e job explicitly uses github.event.pull_request.head.sha to test the PR's actual code

Which issue(s) this PR is related to:

Fixes #793

Special notes for your reviewer:

The pull_request_target event runs the workflow definition from the base branch (main), not the PR branch. This is the standard secure pattern for running CI with secrets on fork PRs — the workflow itself is trusted (from main), and the ok-to-test label ensures maintainer approval before secrets are used.

Security considerations addressed:

  • pull_request_target only triggers on labeled (not synchronize), requiring maintainers to re-apply ok-to-test after each new commit from a fork
  • Concurrency groups include event name to prevent pull_request and pull_request_target runs from cancelling each other

Does this PR introduce a user-facing change?

NONE

GitHub Actions does not expose repository secrets to pull_request events
from forks. Switch the e2e job to trigger on pull_request_target instead,
which runs in the base repository context and has access to secrets. The
existing ok-to-test label gate ensures maintainer review before secrets
are exposed. Non-e2e jobs continue to use pull_request to avoid double
runs. The checkout step now explicitly uses the PR head SHA to test the
correct code.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 1 file

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name=".github/workflows/ci.yaml">

<violation number="1" location=".github/workflows/ci.yaml:9">
P1: Adding `pull_request_target` without separating concurrency groups can cause `pull_request` and `pull_request_target` runs to cancel each other, potentially skipping required CI jobs.</violation>

<violation number="2" location=".github/workflows/ci.yaml:11">
P0: `pull_request_target` on `synchronize` allows newly pushed fork commits to run e2e with secrets as long as a prior `ok-to-test` label remains. Require fresh maintainer re-approval per commit (for example, trigger only on `labeled`).</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

@gjkim42
Copy link
Collaborator

gjkim42 commented Mar 26, 2026

/kelos pick-up

@kelos-bot
Copy link
Author

kelos-bot bot commented Mar 26, 2026

🤖 Kelos Task Status

Task kelos-pr-responder-796 has succeeded. ✅

- Restrict pull_request_target to only 'labeled' type so maintainers must
  re-apply ok-to-test after each new fork commit (prevents stale label reuse)
- Include event_name in concurrency group key to prevent pull_request and
  pull_request_target runs from cancelling each other

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@kelos-bot kelos-bot bot added the kind/bug Categorizes issue or PR as related to a bug label Mar 26, 2026
@github-actions github-actions bot removed the needs-kind Indicates an issue or PR lacks a kind/* label label Mar 26, 2026
@kelos-bot
Copy link
Author

kelos-bot bot commented Mar 26, 2026

🤖 Kelos Agent @gjkim42

/kelos needs-input

All review feedback has been addressed:

  • P0: Restricted pull_request_target to only labeled type, requiring maintainers to re-apply ok-to-test after each new fork commit
  • P1: Added event_name to concurrency group key to prevent pull_request and pull_request_target runs from cancelling each other

Ready for re-review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

e2e tests cannot access secrets for PRs from forked repositories

1 participant